Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use ColumnDef position in lint report #391

Merged
merged 2 commits into from
Nov 12, 2024
Merged

Use ColumnDef position in lint report #391

merged 2 commits into from
Nov 12, 2024

Conversation

ermakov-oleg
Copy link
Contributor

Summary

Fixed the display of lint errors for large DDL statements. Previously, when an error occurred, the output included the entire SQL statement, which could be very lengthy. The update now shows only the relevant line(s) with the issue.

Problem

For large DDL files, any linting errors resulted in the full SQL being displayed in the output, making it difficult to identify the exact problematic lines in lengthy scripts.

Example File:

create table test_table (
    col1 varchar(255),
    col2 varchar(255),
    col3 varchar(255)
    --- other columns
);

Previous Output

Before the change, the output included the full SQL statement, repeated for each warning:

.tmp/big-int.sql:1:0: warning: prefer-text-field

   1 | create table test_table (
   2 |     col1 varchar(255),
   3 |     col2 varchar(255),
   4 |     col3 varchar(255)
   5 |     --- other columns
   6 | );

  note: Changing the size of a varchar field requires an ACCESS EXCLUSIVE lock.
  help: Use a text field with a check constraint.

.tmp/big-int.sql:1:0: warning: prefer-text-field

   1 | create table test_table (
   2 |     col1 varchar(255),
   3 |     col2 varchar(255),
   4 |     col3 varchar(255)
   5 |     --- other columns
   6 | );

  note: Changing the size of a varchar field requires an ACCESS EXCLUSIVE lock.
  help: Use a text field with a check constraint.

.tmp/big-int.sql:1:0: warning: prefer-text-field

   1 | create table test_table (
   2 |     col1 varchar(255),
   3 |     col2 varchar(255),
   4 |     col3 varchar(255)
   5 |     --- other columns
   6 | );

  note: Changing the size of a varchar field requires an ACCESS EXCLUSIVE lock.
  help: Use a text field with a check constraint.

find detailed examples and solutions for each rule at https://squawkhq.com/docs/rules
Found 3 issues in 1 file (checked 1 source file)

New Output

After the update, only the relevant lines are displayed, providing a cleaner and more focused output:

.tmp/big-int.sql:2:0: warning: prefer-text-field

   2 | col1 varchar(255),

  note: Changing the size of a varchar field requires an ACCESS EXCLUSIVE lock.
  help: Use a text field with a check constraint.

.tmp/big-int.sql:3:0: warning: prefer-text-field

   3 | col2 varchar(255),

  note: Changing the size of a varchar field requires an ACCESS EXCLUSIVE lock.
  help: Use a text field with a check constraint.

.tmp/big-int.sql:4:0: warning: prefer-text-field

   4 | col3 varchar(255)

  note: Changing the size of a varchar field requires an ACCESS EXCLUSIVE lock.
  help: Use a text field with a check constraint.

find detailed examples and solutions for each rule at https://squawkhq.com/docs/rules
Found 3 issues in 1 file (checked 1 source file)

Additional Information

This improvement significantly reduces the noise in the output, making it easier to identify issues in large SQL files by only showing the lines related to the warning or error.

Let me know if you’d like any changes!

@netlify Netlify
Copy link

netlify bot commented Nov 11, 2024 •
edited
Loading

👷 Deploy request for squawkhq pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 993a450

@ermakov-oleg
Copy link
Contributor Author

Pinned the version of maturin where building the Python wheel was still working.

The build breaks when updating cargo-zigbuild from v0.18.4 to v0.19.4, as it somehow stops working properly with sccache.

I’ll investigate this further in a separate PR later.

sbdchd
sbdchd approved these changes Nov 12, 2024
View reviewed changes
Copy link
Owner

@sbdchd sbdchd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks much better, thank you!

@sbdchd sbdchd added the automerge automerge with kodiak label Nov 12, 2024
@ermakov-oleg
Copy link
Contributor Author

Is something wrong with the bot, and is it unable to merge the changes?

@chdsbd chdsbd merged commit 8a4d0ad into sbdchd:master Nov 12, 2024
24 checks passed
@ermakov-oleg ermakov-oleg deleted the line-number branch November 12, 2024 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbdchd sbdchd sbdchd approved these changes

@chdsbd chdsbd chdsbd approved these changes

Assignees
No one assigned
Labels
automerge automerge with kodiak
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ermakov-oleg @chdsbd @sbdchd